Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose pre-state Version via TransactEx #263

Merged
merged 3 commits into from
Feb 20, 2021
Merged

Expose pre-state Version via TransactEx #263

merged 3 commits into from
Feb 20, 2021

Conversation

bartelink
Copy link
Collaborator

@bartelink bartelink commented Nov 15, 2020

At present, the Equinox.Decider interface only exposes the outgoing Version, which means the only way to determine the incoming Version is by indirect means such as:

  • checking the Index of the freshest event (can be incorrect if an Event that's not in the DU is encountered)
  • inferring it based on the post-Version passed to the mapResult (won't work well with transmute operations or rolling snapshots)

This proposal provides the incoming extended state via a new TransactEx API, enabling one to process the value directly.

As this is only for V3, it makes sense to remove TransactAsyncEx as both APIs are likely extremely rare, and having 2 mysteriously named functions with complex signatures is rarely better than one.

cc @dharmaturtle

@bartelink bartelink added this to the 3.0 milestone Nov 15, 2020
src/Equinox/Equinox.fs Outdated Show resolved Hide resolved
@bartelink
Copy link
Collaborator Author

This PR is an example to illustrate an approach in a discussion on the DDD-CQRS-ES Slack - my instinct is not to add this until we validate a scenario for which this truly is the best solution.

@bartelink
Copy link
Collaborator Author

@dharmaturtle I'll likely take the cleanup aspects of this PR, and the FAQ from this and merge it after #272 for inclusion in the V3 RC

As noted previously, any concept that gets exposed in the public API has a cost in terms of adding stuff for readers of the code to traverse. That doesn't always make it a bad thing - sometimes naming or mentioning the concept can help people's thinking even if they opt not to actually use the facility in the end.

Please let me know if you've found a use case for needing to know the pre-version or have any other thoughts on this.

@dharmaturtle
Copy link
Contributor

I've thrown pre-release binaries into source control before. I don't think it's a bad practice, and neither does Mark Seemann. There's no particular rush :)

@bartelink
Copy link
Collaborator Author

bartelink commented Feb 16, 2021

Thats an interesting idea (though I think Mark's point is different - he'd still be installing the package from nuget, no?); in this instance it's more a concern of whether the feature should be added in the first instance than the release vehicle itself though.

If you wanted/needed to trial it, PR builds yield [alpha-badged] artifacts and can be released, though we try not to do that too often in order to avoid stressing nuget's performance.

On re-reading, I'm beginning to like the OP suggestion of exposing it as a breaking change in TransactAsyncEx - that provides the escape hatch just in case, without suggesting that it's part of any mainline API.

@bartelink bartelink marked this pull request as ready for review February 19, 2021 23:55
@bartelink bartelink changed the base branch from master to tidy-context February 20, 2021 03:22
Base automatically changed from tidy-context to master February 20, 2021 03:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants